Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed oauth2 token handling, populated User in security context. #188

Merged
merged 1 commit into from
Apr 4, 2016

Conversation

jtk54
Copy link
Contributor

@jtk54 jtk54 commented Apr 1, 2016

This PR removes the propagation of OAuth2 tokens between Deck and Gate, and populates the User object returned from the authEndpoint call ('/oauth/info') with the email of the Google User associated with the OAuth2 token granted in the redirect flow. It also factors out Netflix's OAuth2 configuration from the generic configuration.

@jtk54
Copy link
Contributor Author

jtk54 commented Apr 1, 2016

@ttomsu

@@ -103,57 +103,58 @@ class OAuth2SecurityConfig implements WebSecurityAugmentor {
@Override
void configure(AuthenticationManagerBuilder authenticationManagerBuilder) {
authenticationManagerBuilder.authenticationProvider(
googleAuthenticationProvider(googleResourceServerTokenServices())
// googleAuthenticationProvider(googleResourceServerTokenServices())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No dead code - remove it if it's not usable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@jtk54 jtk54 force-pushed the google-oauth-user branch from 552aa7e to 95ac22e Compare April 4, 2016 16:43
@Autowired
AnonymousAccountsService anonymousAccountsService

@Value('${services.deck.baseUrl:http://localhost/9000}')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you want localhost:9000

Also, does this mean that it will always redirect to the homepage? Doesn't the dance in deck encode the requested link?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Yes, this would redirect to deck's homepage each time, which then makes the call to '/oauth/info' again, which is successful the second time (returns the User object).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed the default url string for deck.

This PR completes the proof of concept authentication implementation
using Google as an OAuth2 provider. The user authenticates with Google,
then we use the resulting token to make a user info call, and populate
the session-based SecurityContext with that information.
@jtk54 jtk54 force-pushed the google-oauth-user branch from 95ac22e to 6448340 Compare April 4, 2016 17:18
@ttomsu ttomsu merged commit 5dc00e8 into spinnaker:google-oauth Apr 4, 2016
@jtk54 jtk54 deleted the google-oauth-user branch April 4, 2016 18:16
abhinaybyrisetty referenced this pull request in OpsMx/gate Mar 18, 2021
OP-5567 : Added new request param for AP test-verification endpoint
@spinnakerbot spinnakerbot mentioned this pull request Jun 28, 2021
@spinnakerbot spinnakerbot mentioned this pull request Jul 6, 2021
@spinnakerbot spinnakerbot mentioned this pull request Oct 12, 2022
@spinnakerbot spinnakerbot mentioned this pull request Nov 16, 2022
kirangodishala pushed a commit to kirangodishala/gate that referenced this pull request May 23, 2023
@spinnakerbot spinnakerbot mentioned this pull request May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants